Fix on Discriminator Deserialization Issues#488
Closed
mobias17 wants to merge 4 commits intobinance:masterfrom
Closed
Fix on Discriminator Deserialization Issues#488mobias17 wants to merge 4 commits intobinance:masterfrom
mobias17 wants to merge 4 commits intobinance:masterfrom
Conversation
Author
|
Closing due to improvements in binance-sdk-spot 7.0.0 and binance-common 3.5.0. Evaluating whether new PR is required. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Addressed
Fixes #476, #477, #405
Problem Description
The generated OpenAPI models using Pydantic's
oneOfschema were failing to properly deserialize JSON data. This affected multiple model types across both REST API and WebSocket API implementations:AssetFilters,ExchangeFilters,SymbolFilters) - ThefilterTypefield was not being used as a discriminator, causing deserialization to fail or be extremely inefficientUserDataStreamEventsResponse) - Theefield (event type) was not routing to the correct event schemaRoot Cause: The auto-generated
from_dict()methods attempted to validate against all possible schemas sequentially (O(n) complexity), andmodel_validate()bypassed the custom discriminator logic entirely, leavingactual_instanceasNone.Changes Made
1. Discriminator-Based Routing
Added discriminator maps to all oneOf models:
filterTypefield to directly instantiate the correct filter classrest_api.models.asset_filters(1 filter type)rest_api.models.exchange_filters(4 filter types)rest_api.models.symbol_filters(16 filter types)websocket_api.models.asset_filters(1 filter type)websocket_api.models.exchange_filters(4 filter types)websocket_api.models.symbol_filters(16 filter types)efield to route to correct event typewebsocket_api.models.user_data_stream_events_response(6 event types)2. model_validate() Override
Added
model_validate()classmethod to all oneOf models to delegate to discriminator logic:This ensures both
Model.from_dict()andModel.model_validate()use the same efficient discriminator routing.3. Parent Model Support
Added
model_validate()overrides to parent models in the deserialization chain (7 files):ExchangeInfoResponse,ExchangeInfoResponseSymbolsInner,RateLimitsExchangeInfoResponse,ExchangeInfoResponseResult,ExchangeInfoResponseResultSymbolsInner,ExchangeInfoResponseResultSorsInner,RateLimitsUpdated their
from_dict()methods to useBaseModel.model_validate.__func__(cls, obj)to avoid infinite recursion while properly deserializing nested oneOf fields.4. Legacy Code Removal
Removed 300+ lines of inefficient sequential validation code that attempted to try each schema in order.
Why This Works
from_dict()andmodel_validate()now work identicallyBaseModel.model_validate.__func__()calls Pydantic's native validation, avoiding infinite loopsTesting
All existing tests pass (54 tests total):
Validated discriminator routing for:
Comparison to PR #423
Unlike PR #423 which only addressed
UserDataStreamEventsResponseby wrapping input data with{"actual_instance": ...}, this PR:Review Notes
Key files to review:
*_filters.pyfile - check discriminator map completenessuser_data_stream_events_response.py- verify event type routingmodel_validateoverrides - ensure no recursionTesting suggestion: